Skip to content

perf!: reduce hot-path allocations in receipt, send, and signal paths - #519

Merged
jlucaso1 merged 1 commit into
mainfrom
perf/hot-path-allocation-reduction
Apr 12, 2026
Merged

perf!: reduce hot-path allocations in receipt, send, and signal paths#519
jlucaso1 merged 1 commit into
mainfrom
perf/hot-path-allocation-reduction

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Apr 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Receipt construction: move values instead of cloning — reduces from 6 clones to 1 per incoming receipt
  • Remove dead Receipt.message_sender: field duplicated source.sender and was never read
  • encode_and_pad(): replace pad_message_v2(msg.encode_to_vec()) with single pre-sized allocation in DM, group SKDM, and signal encrypt paths
  • Remove redundant clone: jid_to_encryption_jid.insert(jid, jid.clone()) was unnecessary — the encrypt loop already falls back to device_jid when no mapping exists
  • Remove unused import: prost::Message no longer needed in signal.rs

Breaking changes

  • Receipt.message_sender field removed (was never accessed, always duplicated source.sender)

Experiment data

From experiments/heaptrack-impact-lab (10k node benchmark):

Optimization alloc delta byte delta time delta
Receipt: move vs clone -100% -100% -72.4%
encode_and_pad vs encode_to_vec+pad -99.9% -98.5% ~neutral
Session map: skip self-mapping +0% +0% -4.9%

Test plan

  • cargo fmt --all
  • cargo clippy --all --tests
  • cargo test --all --exclude e2e-tests

Summary by CodeRabbit

  • Breaking Changes

    • Removed message_sender field from Receipt events. Code relying on this field requires updates.
  • Refactor

    • Optimized message encryption plaintext preparation across direct and group messaging paths.
    • Enhanced Receipt handling to eliminate redundant copying and improve memory efficiency.
    • Removed unused code dependencies.

- Receipt: move values instead of cloning (6 clones → 1 per receipt)
- Remove dead `Receipt.message_sender` field (duplicated `source.sender`)
- Use `encode_and_pad()` instead of `pad_message_v2(encode_to_vec())`
  for single pre-sized allocation in DM, group SKDM, and signal paths
- Remove redundant self-mapping clone in encrypt_for_devices
- Remove unused prost import from signal.rs
@coderabbitai

coderabbitai Bot commented Apr 12, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9f16c974-1217-4c1a-a41c-718f34ca2ed6

📥 Commits

Reviewing files that changed from the base of the PR and between 1007676 and 2f05e91.

📒 Files selected for processing (4)
  • src/features/signal.rs
  • src/receipt.rs
  • wacore/src/send.rs
  • wacore/src/types/events.rs
💤 Files with no reviewable changes (1)
  • wacore/src/types/events.rs

📝 Walkthrough

Walkthrough

This PR refactors message encryption handling and receipt type management across the codebase. Changes include replacing MessageUtils::pad_message_v2(message.encode_to_vec()) with MessageUtils::encode_and_pad(message) for more efficient plaintext preparation, removing redundant cloning in receipt construction to preserve ownership semantics, and removing the message_sender field from the public Receipt struct (a breaking change).

Changes

Cohort / File(s) Summary
Message Encryption Refactoring
src/features/signal.rs, wacore/src/send.rs
Replaced MessageUtils::pad_message_v2(message.encode_to_vec()) calls with MessageUtils::encode_and_pad(message) for DSM and SKDM wrapper messages. Also removed a jid_to_encryption_jid insertion when a session already exists under the direct device address in encrypt_for_devices.
Receipt Type and Field Changes
src/receipt.rs, wacore/src/types/events.rs
Refactored handle_receipt to use ownership-preserving patterns (moving values instead of cloning); removed public message_sender: Jid field from Receipt struct. Updated downstream receipt field references to use the constructed receipt object.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • #474: Modifies signal/message encryption codepaths with the same MessageUtils::pad_message_v2 to MessageUtils::encode_and_pad refactoring pattern.
  • #502: Applies similar encode-and-pad changes and ownership-preserving ciphertext serialization patterns across signal and send paths.
  • #362: Touches related messaging/receipt and send-path code with overlapping MessageUtils and Receipt type changes.

Suggested labels

breaking-change

Poem

🐰 Hops through plaintext with swiftened stride,
encode\_and\_pad now takes the ride,
Clones replaced with moves so clean,
Receipt fields trimmed, leaner seen!
Breaking change hops past with care,
Ownership patterns fill the air!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main focus of the changeset: performance optimization through allocation reduction in three key paths (receipt, send, and signal).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/hot-path-allocation-reduction

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jlucaso1

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Apr 12, 2026

Copy link
Copy Markdown

Claude finished @jlucaso1's task in 1m 57s —— View job


Code Review: perf!: reduce hot-path allocations

  • Read changed files and diff
  • Analyze changes for correctness, safety, and style
  • Post review feedback

Verdict: Approve — clean, correct, and well-motivated changes. All four optimizations are sound. A couple of minor observations below, but nothing blocking.


1. Receipt construction — move semantics (src/receipt.rs)

The refactor from 6 clones to moves is correct. Key details verified:

  • is_group is computed from from.is_group() before from is moved into receipt.source.chat (line 49 vs 59) — good.
  • Post-move accesses (receipt.r#type, receipt.source.chat) correctly go through the struct — no use-after-move.
  • ReceiptType is Clone + PartialEq (not Copy), so the comparison receipt.r#type == ReceiptType::Retry borrows through the struct — correct.

One minor note: ReceiptType is a simple fieldless enum. It could derive Copy to avoid the borrow concern entirely, but that's outside this PR's scope and not a problem as-is.

2. Receipt.message_sender removal (wacore/src/types/events.rs)

Confirmed no references to Receipt.message_sender exist anywhere in the codebase — grep returns zero hits. The field was always a duplicate of source.sender. The breaking change callout in the PR description is appropriate since Receipt is a public struct with Serialize.

3. encode_and_pad() (src/features/signal.rs, wacore/src/send.rs)

The switch from pad_message_v2(msg.encode_to_vec()) to encode_and_pad(msg) eliminates a double allocation:

  • encode_to_vec() allocates a Vec, then pad_message_v2 resizes it (potentially reallocating if padding exceeds spare capacity).
  • encode_and_pad pre-sizes Vec::with_capacity(encoded_len + pad), encodes in-place, then resize is a no-op reallocation since capacity was pre-reserved.

All three call sites (signal encrypt, DM prepare_dm_stanza, group SKDM) are updated consistently.

The unused prost::Message import removal in signal.rs is correct — encode_to_vec() was the only thing requiring it.

4. Redundant self-mapping removal (wacore/src/send.rs:405)

The removed line was:

jid_to_encryption_jid.insert(device_jid, device_jid.clone());

Both consumers of jid_to_encryption_jid use .get(device_jid).unwrap_or(device_jid) (lines 483, 605), which already falls back to the device JID itself when no mapping exists — making the self-insert a no-op allocation. Verified that this branch (continue at line 406) means the device already has a session under its direct address and won't appear in jids_needing_prekeys, so the line 483 consumer is also unaffected. Correct removal.

The two removed comment lines (// Session exists under direct address, use it) matched the deleted logic — clean.


LGTM. Nice targeted perf wins with a small diff.

@github-actions

Copy link
Copy Markdown

Benchmark Results

59 unchanged benchmark(s)
Benchmark Current Baseline Change
reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message() 3,879 3,879 +0.0%
reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message() 11,851 11,851 +0.0%
reporting_token_benchmark::key_derivation_group::bench_key_derivation 43,398 43,398 +0.0%
reporting_token_benchmark::token_calculation_group::bench_token_calculation 19,365 19,365 +0.0%
reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple() 68,814 68,814 +0.0%
reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended() 76,781 76,781 +0.0%
reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message() 2,214 2,214 +0.0%
reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message() 5,939 5,939 +0.0%
send_receive_benchmark::dm_send::bench_dm_send text:setup_dm_send() 177,556 177,971 -0.2%
send_receive_benchmark::dm_recv::bench_dm_recv text:setup_dm_recv() 191,784 191,870 -0.0%
send_receive_benchmark::group_send::bench_group_send group_10:setup_group_send_10() 888,990 888,986 +0.0%
send_receive_benchmark::group_send::bench_group_send group_50:setup_group_send_50() 980,393 980,188 +0.0%
send_receive_benchmark::group_send::bench_group_send group_256:setup_group_send_256() 1,465,403 1,465,398 +0.0%
send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_10:setup_group_skdm_10() 2,721,818 2,744,114 -0.8%
send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_50:setup_group_skdm_50() 10,056,630 10,141,275 -0.8%
send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_256:setup_group_skdm_256() 47,874,720 48,311,928 -0.9%
send_receive_benchmark::group_recv::bench_group_recv text:setup_group_recv() 12,695,493 12,685,242 +0.1%
binary_benchmark::marshal_group::bench_marshal_allocating 95,585 95,585 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_allocating 95,618 95,618 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_allocating 113,974 113,974 +0.0%
binary_benchmark::marshal_group::bench_marshal_reusing_buffer 102,895 102,895 +0.0%
binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer 95,685 95,685 +0.0%
binary_benchmark::marshal_group::bench_marshal_long_string 15,762 15,762 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_long_string 15,806 15,806 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_long_string 17,592 17,592 +0.0%
binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating 533,122 533,122 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating 532,688 532,688 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating 534,046 534,046 +0.0%
binary_benchmark::marshal_group::bench_marshal_many_children_allocating 13,413,012 13,413,012 +0.0%
binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating 13,357,245 13,357,245 +0.0%
binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating 26,652,528 26,652,528 +0.0%
binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled() 2,498 2,498 +0.0%
binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled() 38,500 38,500 +0.0%
binary_benchmark::unpack_group::bench_unpack_uncompressed 773 773 +0.0%
binary_benchmark::unpack_group::bench_unpack_compressed 556,090 556,090 +0.0%
binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled() 5,039 5,039 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled() 7,484 7,484 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled() 90,824 90,824 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled() 7,511 7,511 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled() 90,860 90,860 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled() 8,838 8,838 +0.0%
binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled() 104,670 104,670 +0.0%
binary_benchmark::child_iteration_group::bench_get_children_by_tag 475,970 475,970 +0.0%
binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled() 13,493 13,493 +0.0%
libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users() 17,382,958 17,187,816 +1.1%
libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session() 160,994 160,994 +0.0%
libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message() 5,511,244 5,511,244 +0.0%
libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session() 161,731 161,731 +0.0%
libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender() 298,353 298,353 +0.0%
libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution() 712,883 712,883 +0.0%
libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message() 12,586,974 12,455,803 +1.1%
libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data() 27,535,022 27,610,863 -0.3%
libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message() 3,467,011 3,467,011 +0.0%
libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message() 124,581,943 125,771,623 -0.9%
libsignal_benchmark::signature_group::bench_key_generation keygen 2,830,493 2,830,493 +0.0%
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions() 46,003 46,003 +0.0%
libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages() 5,105,272 5,105,272 +0.0%
libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session() 297,627 297,627 +0.0%
libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction() 14,247,117 14,247,117 +0.0%
No significant changes detected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant